[Improv]: enable max ordinal check after write state engine prepare write; by azeng-netflix · Pull Request #787 · Netflix/hollow

azeng-netflix · 2026-01-21T05:11:08Z

Summary:
This change adds configurable soft limit validation to ByteArrayOrdinalMap and exposes ordinal map
statistics to PublishListeners for monitoring.

Changes:

Soft Limit Enforcement
- Adds soft limit threshold at 268,435,456 (2^28) - half of the hard limit
- Throws exception when soft limit exceeded (configurable behavior)
- Override mechanism via HollowProducer.withIgnoreSoftLimits(Supplier)
- One-time logging per cycle to prevent log spam
Stats Exposure
- New ByteArrayOrdinalMapStats class captures maxOrdinal, byteDataLength, and loadFactor
- New PublishStageStats wraps stats for all type write states
- Extended PublishListener.onPublishComplete() to receive stats (backward compatible)
- Stats collected after successful publish and passed through listener chain
3. Perf Improvement
- Remove redundant scan of pointersAndOrdinals to find the max ordinal in HollowWriteTypeState. Instead, find and return the max ordinal when doing prepareForWrite

Backward Compatibility:

Default constructor maintains existing behavior (soft limits disabled by default)
Deprecated onPublishComplete method preserved with default implementation
No breaking changes to existing API

hollow/src/main/java/com/netflix/hollow/core/write/HollowTypeWriteState.java

Sunjeet · 2026-01-28T18:36:33Z

hollow/src/main/java/com/netflix/hollow/api/producer/HollowProducer.java

+         * @param ignoreOrdinalThresholdBreach the supplier that returns true to ignore threshold breaches
+         * @return this builder
+         */
+        public B withIgnoreOrdinalThresholdBreachConfig(Supplier<Boolean> ignoreOrdinalThresholdBreach) {


Can make this more generic to all soft limits- ignoreSoftLimits

Sunjeet · 2026-01-28T18:38:30Z

hollow/src/main/java/com/netflix/hollow/core/memory/ByteArrayOrdinalMap.java

    private static final long ORDINAL_MASK = (1L << BITS_PER_ORDINAL) - 1;
    private static final long MAX_BYTE_DATA_LENGTH = 1L << BITS_PER_POINTER;
+    // a safe limit for the ordinal. Even though the ordinal value range is [0, (1<<29) - 1], in production,
+    // the user should be warned whenever the ordinal value exceeds (1<<28) - 1.


nit: drop "in production" and replace "the user should be warned" with something like- fail the producer cycle unless soft limits are overridden

Sunjeet · 2026-01-28T18:48:39Z

hollow/src/main/java/com/netflix/hollow/core/memory/ByteArrayOrdinalMap.java


+        if (ordinal >= SOFT_ORDINAL_LIMIT) {
+            if (logOrdinalThresholBreach && ignoreOrdinalThresholdBreach) {
+                logOrdinalThresholBreach = false;


nit: typo in logOrdinalThresholBreach - but I guess it will be renamed to logSoftLimitBreach anyway

Sunjeet · 2026-01-28T18:59:47Z

hollow/src/main/java/com/netflix/hollow/core/write/HollowTypeWriteState.java

     */
    public void prepareForNextCycle() {
        ordinalMap.compact(currentCyclePopulated, numShards, stateEngine.isFocusHoleFillInFewestShards());
+        ordinalMap.setIgnoreOrdinalThresholdBreach(ignoreOrdinalThresholdBreachSupplier == null ? false : ignoreOrdinalThresholdBreachSupplier.get());


here and anywhere else where there isn't a way to override the soft limit failure, lets default to not failing on soft limits

Sunjeet · 2026-01-28T19:00:44Z

hollow/src/main/java/com/netflix/hollow/api/producer/listener/PublishListener.java

     */
    void onBlobPublish(Status status, HollowProducer.Blob blob, Duration elapsed);

+    default void onPublishHeaderBlob(HollowWriteStateEngine writeStateEngine) {


This is used for listener to do customized report

coupling with onPublishHeaderBlob doesn't feel great, can we explore reusing the onPopulateComplete listener method?

I have moved it to onPublishComplete.
The reason of not doing it in onPopulateComplete, is that the WriteStateEngine has not been prepared for write yet.

azeng-netflix · 2026-01-30T21:41:35Z

@azeng-netflix Before merge, check again to see if missing any places where the limit supplier are not properly propagated/copied over.

azeng-netflix · 2026-01-30T21:43:09Z

hollow/src/test/java/com/netflix/hollow/api/producer/HollowProducerTest.java

    }

+     // check if ByteArrayOrdinalMap.ignoreSoftLimits has been updated to the expected value.
+    private void checkBaomIgnoreOrdinalSoftLimit(boolean expected, HollowProducer producer) {


NOTE: Modified this test file to check if ignore ordinal limit can be updated between cycles/restore

Sunjeet · 2026-02-02T13:58:59Z

hollow/src/main/java/com/netflix/hollow/core/write/HollowTypeWriteState.java

+     *
+     * @return a {@link ByteArrayOrdinalMapStats} containing map statistics
+     */
+    public ByteArrayOrdinalMapStats getOrdinalMapStatsAfterPreparation() {


nit: could simplify name to getStats , the clarification you have in javadoc feels sufficient

Changed to getOrdinalMapStats in case in the future other stats also need to be exposed from HollowTypeWriteState

Sunjeet · 2026-02-02T14:02:22Z

hollow/src/main/java/com/netflix/hollow/core/write/HollowWriteStateEngine.java

    //// adjust number of shards per type during the course of the delta chain to realize consumer-side delta applications at constant space overhead
    private boolean allowTypeResharding = false;
+    //// supplier to determine whether to ignore ordinal threshold breaches (log warning instead of throwing exception)
+    private Supplier<Boolean> ignoreOrdinalSoftLimitsSupplier;


nit: name this more generic ignoreSoftLimitsSupplier, becuase we would reuse it for any other limits. Could also drop supplier from the name

Changed to ignoreSoftLimits

Sunjeet · 2026-02-02T14:07:45Z

hollow/src/main/java/com/netflix/hollow/core/memory/ByteArrayOrdinalMap.java

+                }
+                // continue silently after first log
+            } else {
+                String message = String.format("Ordinal %d exceeds the soft ordinal limit of %d.",


nit: adding a hint in the log msg about support for ignoring soft limits could be useful

Sunjeet · 2026-02-02T14:10:54Z

hollow/src/main/java/com/netflix/hollow/core/write/HollowListTypeWriteState.java

        this(schema, -1);
    }
+
+    public HollowListTypeWriteState(HollowListSchema schema, Supplier<Boolean> ignoreOrdinalSoftLimitsSupplier) {


I dont think we really need this new constructor, could just reuse the other one you added?

Sunjeet · 2026-02-02T14:11:51Z

hollow/src/main/java/com/netflix/hollow/core/write/HollowListTypeWriteState.java

+    }

    public HollowListTypeWriteState(HollowListSchema schema, int numShards) {
        super(schema, numShards);


nit: changing to this(schema, numShards, null) would be a bit more readable

Sunjeet · 2026-02-02T14:22:55Z

hollow/src/main/java/com/netflix/hollow/api/producer/listener/PublishListener.java

+     * @param version version that was published.
+     * @param elapsed duration of the publish stage in {@code unit} units
+     */
+    default void onPublishComplete(HollowWriteStateEngine writeStateEngine, Status status, long version, Duration elapsed) {


An alternative would be to add a Stats within the State object, where different stages like publish, etc. could implement their stats like PublishStage could implement stats like duration which could be generic to all stages, or specific ones like ordinap map stats for publish stage. If you think it fits well, otherwise the current approach seems fine too, but if we're doing this then lets also deprecate the older 3 param onPublishComplete, and make this default impl call that one. Could also add to the javadoc- The default implementation delegates to the deprecated 3 param onPublishComplete for backwards compatibility.

Made this commit to address the comment, but during the implementation, I think a general Stats would require the Listener side to do type casting like stats instanceof PublishStageStats. would like your opinion on this.

add and use PublishListener.onPublishComplete(Status status, long version, Duration elapsed, PublishStageStats stats)

…poses ordinal map statistics to PublishListeners for monitoring.

azeng-netflix force-pushed the azeng-max-ordinal-check branch from 7745b1a to 53aef77 Compare January 23, 2026 22:02

azeng-netflix commented Jan 23, 2026

View reviewed changes

hollow/src/main/java/com/netflix/hollow/core/write/HollowTypeWriteState.java Outdated Show resolved Hide resolved

azeng-netflix force-pushed the azeng-max-ordinal-check branch from 690a96d to f30e000 Compare January 27, 2026 06:05

azeng-netflix mentioned this pull request Jan 27, 2026

[Improv]: max ordinal check, rely on Listener; #789

Closed

Sunjeet reviewed Jan 28, 2026

View reviewed changes

azeng-netflix force-pushed the azeng-max-ordinal-check branch 3 times, most recently from 308129f to a80d8cc Compare January 30, 2026 01:00

azeng-netflix marked this pull request as ready for review January 30, 2026 17:54

azeng-netflix commented Jan 30, 2026

View reviewed changes

Sunjeet reviewed Feb 2, 2026

View reviewed changes

azeng-netflix force-pushed the azeng-max-ordinal-check branch 3 times, most recently from 4952801 to 9f0ef02 Compare February 3, 2026 23:46

Sunjeet approved these changes Feb 4, 2026

View reviewed changes

Adds configurable soft limit validation to ByteArrayOrdinalMap and ex…

775b98a

…poses ordinal map statistics to PublishListeners for monitoring.

azeng-netflix force-pushed the azeng-max-ordinal-check branch from 9f0ef02 to 775b98a Compare February 4, 2026 23:38

azeng-netflix merged commit d417781 into master Feb 5, 2026
2 checks passed

Conversation

azeng-netflix commented Jan 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

azeng-netflix commented Jan 30, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

azeng-netflix Feb 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

azeng-netflix commented Jan 21, 2026 •

edited

Loading

azeng-netflix Feb 2, 2026 •

edited

Loading